Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit database after creating oauth_tokens table. Fixes #75 #76

Closed
wants to merge 2 commits into from

Conversation

hunterji
Copy link
Contributor

This change seems to be required in Python 3.6.0. It works correctly without the db.commit() in 3.5.2 and earlier. Without the commit the first table, oauth_cache_db_version, gets created, but oauth_tokens does not.

I ran the unit tests on 3.5.2 after making the change and all passed. I couldn't run them on 3.6.0 because pytest isn't supported there yet. I did make the change and test manually in 3.6.0. All tests were on Windows 10.



This change seems to be required in Python 3.6.0. It works correctly without the db.commit() in 3.5.2 and earlier. Without the commit the first table, oauth_cache_db_version, gets created, but oauth_tokens does not.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 67.571% when pulling e6c8004 on hunterji:master into 142d199 on sybrenstuvel:master.

@thijstriemstra
Copy link
Collaborator

@hunterji can you also add python 3.6 to tox/travis?

@hunterji
Copy link
Contributor Author

I added py36 to tox.ini

@hunterji
Copy link
Contributor Author

Adding py36 causes TravisCI to fail the checks. Is this because pytest doesn't support Python 3.6 yet? That is why I hadn't added py36 to tox.ini, but I've never used TravisCI so I'm just guessing.

@sybrenstuvel
Copy link
Owner

To get TravisCI to play along, you also need to change .travis.yml; it's a good idea to edit that one too. However, it is not the cause of the error that TravisCI is showing:

VersionConflict: (six 1.9.0 (/home/travis/build/sybrenstuvel/flickrapi/.tox/pypy/site-packages), Requirement.parse('six>=1.10.0'))

I'm not sure what exactly requires six>=1.10.0, maybe that's the first version that supports Python 3.6?

@sybrenstuvel
Copy link
Owner

Please do only one thing per pull request. That makes it much easier to review & accept.

@sybrenstuvel
Copy link
Owner

Cherry-picked original PR in 0556a7e.

@sybrenstuvel
Copy link
Owner

I've added Python 3.6 to Tox & Travis in 4e256ab, rewriting how we set up Travis in the first place.

Before this commit, Travis multiplied the Python versions with the environments, and ran the tests for each combination. The new approach sets things up correctly, matching Python version with TOXENV variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants